-
Notifications
You must be signed in to change notification settings - Fork 725
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
scheduler: hotspot: add pending influence #1982
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1982 +/- ##
==========================================
+ Coverage 77.35% 77.59% +0.23%
==========================================
Files 178 178
Lines 18149 18240 +91
==========================================
+ Hits 14040 14154 +114
+ Misses 3020 3009 -11
+ Partials 1089 1077 -12
Continue to review full report at Codecov.
|
server/schedulers/hot_region.go
Outdated
} | ||
status := op.Status() | ||
if !operator.IsEndStatus(status) { | ||
return 1, false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why 1?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The operator now is running, so we will estimate that all influence of this region is not observed by statistics.
@@ -229,27 +259,35 @@ func (h *balanceHotRegionsScheduler) balanceHotReadRegions(cluster opt.Cluster) | |||
const balanceHotRetryLimit = 10 | |||
|
|||
func (h *balanceHotRegionsScheduler) balanceHotWriteRegions(cluster opt.Cluster) []*operator.Operator { | |||
balancePeer := h.r.Int()%2 == 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why introduce it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sometimes it will only try one balance strategy. I think that is not what we want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. @rleungx PTAL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rest LGTM.
/run-all-tests |
What problem does this PR solve?
Schedulers may overscheduled because the influence of some operators is not observed.
What is changed and how it works?
This PR introduce "pending influence" which records operators and their influence, then amend the statistics according to operator status and influence.
Check List
Tests